Skip to content

fix: reject null values in dense collections#4543

Open
vcjana wants to merge 5 commits intomainfrom
fix-dense-collection-null-handling
Open

fix: reject null values in dense collections#4543
vcjana wants to merge 5 commits intomainfrom
fix-dense-collection-null-handling

Conversation

@vcjana
Copy link
Contributor

@vcjana vcjana commented Feb 24, 2026

Description

SDK now correctly throws an error when encountering null values in non-sparse (dense) collections during deserialization, instead of silently dropping them.

Changes

  • JSON: Client now throws error on null in dense list/map
  • CBOR: Client now throws error on null in dense collections
  • Added unit tests for dense/sparse null handling

Related

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@vcjana vcjana force-pushed the fix-dense-collection-null-handling branch from a189021 to 9815deb Compare February 24, 2026 02:20
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@vcjana vcjana marked this pull request as ready for review February 24, 2026 08:17
@vcjana vcjana requested review from a team as code owners February 24, 2026 08:17
Copy link
Contributor

@ysaito1001 ysaito1001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix. Didn't realize that rejection logic for "null in dense" has existed for server SDK.

FailingTest.RequestTest(REST_XML, "HttpEmptyPrefixHeadersRequestClient"),
FailingTest.RequestTest(REST_JSON, "RestJsonHttpEmptyPrefixHeadersRequestClient"),
// Smithy protocol tests expect null to be silently dropped, but we now correctly reject it.
// These tests will be removed in https://github.com/smithy-lang/smithy/pull/2972
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good finding!

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Contributor

@landonxjames landonxjames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants